Add EgressRule to NetworkPolicy #51351

cmluciano · 2017-08-25T18:50:55Z

What this PR does / why we need it:
Add EgressRule to NetworkPolicy

Which issue this PR fixes: fixes #50453

Special notes for your reviewer:

Please take a look at the comments for the various types. I tried to mimic some of the language used in the Ingress comments, but I may have mangled some sentences.
Let me know if I should add some test cases for validation. I have 2-3, and did not think it was necessary to replicate each case already covered in ingress.

Release note:

Add egress policies to NetworkPolicy

danwinship · 2017-08-25T19:01:05Z

pkg/apis/networking/types.go

@@ -55,6 +55,16 @@ type NetworkPolicySpec struct {
 	// solely to ensure that the pods it selects are isolated by default)
 	// +optional
 	Ingress []NetworkPolicyIngressRule
+
+	// List of egress rules to be applied to the selected pods. Outgoing traffic is
+	//allowed unbounded if there are no NetworkPolicies selecting the pod


missing space after "//", and the word "unbounded" doesn't seem necessary, and also we need to add the thing discussed in https://groups.google.com/d/msg/kubernetes-sig-network/nWyAL-8vKV8/DDRZ9Jx8BwAJ so that all pods with ingress rules don't suddenly become default-deny to egress

danwinship · 2017-08-25T19:04:07Z

pkg/apis/networking/types.go

+	// List of egress rules to be applied to the selected pods. Outgoing traffic is
+	//allowed unbounded if there are no NetworkPolicies selecting the pod
+	// (and cluster policy otherwise allows the traffic), OR if the traffic source is
+	// the pod's local node, OR if the traffic matches at least one egress rule


"if the traffic source is the pod's local node" is meaningless here. You might have meant to change it to "if the traffic destination is the pod's local node", but I don't think we need that exception here. (It exists in ingress rules to make sure that the kubelet can do health checks on pods.)

I agree we do not need the excption, unless someone can say exactly why we need it.

danwinship · 2017-08-25T19:04:44Z

pkg/apis/networking/types.go

+	// the pod's local node, OR if the traffic matches at least one egress rule
+	// across all of the NetworkPolicy objects whose podSelector matches the pod. If
+	// this field is empty then this NetworkPolicy does not limit any outgoing traffic
+	// (and serves solely to ensure that the pods it selects are isolated by default)


does not allow any outgoing traffic

danwinship · 2017-08-25T19:06:07Z

pkg/apis/networking/types.go

@@ -77,6 +87,26 @@ type NetworkPolicyIngressRule struct {
 	From []NetworkPolicyPeer
 }

+// NetworkPolicyEgressRule describes a particular set of traffic that pods can egress to
+// matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to.


"NetworkPolicyEgressRule describes a particular set of traffic that is allowed out of pods matched by a NetworkPolicySpec's podSelector."

danwinship · 2017-08-25T19:06:20Z

pkg/apis/networking/types.go

+// matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to.
+type NetworkPolicyEgressRule struct {
+	// List of destination ports for outgoing traffic.
+	//Each item in this list is combined using a logical OR. If this field is


(missing space)

danwinship · 2017-08-25T19:07:22Z

pkg/apis/networking/types.go

+	//Each item in this list is combined using a logical OR. If this field is
+	// empty or missing, this rule matches all ports (traffic not restricted by port).
+	// If this field is present and contains at least one item, then this rule does note
+	// constrain outgoing traffic.


"If this field is present and contains at least one item, then this rule allows traffic only if the traffic matches at least one port in the list". (Just copied from NetworkPolicyIngressRule.Ports)

danwinship · 2017-08-25T19:09:57Z

pkg/apis/networking/types.go

+	// source). If this field is present and contains at least on item, this rule
+	// allows traffic only if the traffic matches at least one item in the from list.
+	// +optional
+	To []NetworkPolicyPeer


// List of ~~sources~~ destinations for outgoing traffic of pods selected for this rule.
// Items in this list are combined using a logical OR operation. If this field is
// empty or missing, this rule matches all ~~sources~~ destinations (traffic not restricted by
// ~~source~~ destination). If this field is present and contains at least on one item, this rule
// allows traffic only if the traffic matches at least one item in the ~~from~~ to list.

in the to list. -> in the To list.

danwinship · 2017-08-25T19:13:10Z

pkg/apis/networking/validation/validation.go

+	// Validate egress rules 
+	for i, egress := range spec.Egress {
+		egressPath := fldPath.Child("egress").Index(i)
+		for i, port := range egress.Ports {


Probably should split out ValidateNetworkPolicyPort and ValidateNetworkPolicyPeer functions to share between ingress and egress validation

(You "+1"ed this but then didn't actually do it.)

thockin

looks good overall

thockin · 2017-08-26T04:00:37Z

pkg/apis/networking/types.go

+	// List of egress rules to be applied to the selected pods. Outgoing traffic is
+	//allowed unbounded if there are no NetworkPolicies selecting the pod
+	// (and cluster policy otherwise allows the traffic), OR if the traffic source is
+	// the pod's local node, OR if the traffic matches at least one egress rule


I agree we do not need the excption, unless someone can say exactly why we need it.

thockin · 2017-08-26T04:02:37Z

pkg/apis/networking/types.go

+	// this field is empty then this NetworkPolicy does not limit any outgoing traffic
+	// (and serves solely to ensure that the pods it selects are isolated by default)
+	// +optional
+	Egress []NetworkPolicyEgressRule


Do we have a comment indicating 'beta' or 'alpha' ? @lavalamp since machinery team seems to want people to just use fields rather than annotations, shouldn't the generated docs at least say "beta" ?

Didn't we decide we need a field to discriminate whether this policy was applying ingress vs egress?

Yes @danwinship mentioned the additional PolicyType above. Adding now.

Yes it should be beta, but I was not sure if the format was supposed to be similar to the feature gate note or not.

// owner: @cmluciano // beta: v1.8

I've noticed this doesn't mention local node. What's the behavior for if the traffic destination is the pod's local node?

thockin · 2017-08-26T04:09:24Z

staging/src/k8s.io/api/extensions/v1beta1/types.go

+	// source). If this field is present and contains at least on item, this rule
+	// allows traffic only if the traffic matches at least one item in the from list.
+	// +optional
+	To []NetworkPolicyPeer `json:"to,omitempty" protobuf:"bytes,2,rep,name=to"`


port/to is kind of awkward in this direction. ingress.ports and ingress.from makes sense. I'm not sure I have a better answer off the top of my head, and breaking symmetry would be bad, but ... better ideas?

Does Out sound any better than To?

+1 for ports/out

thockin · 2017-08-26T04:10:58Z

Are we declaring the beta?

cmluciano · 2017-08-26T23:43:05Z

@thockin @danwinship @caseydavenport

I've been pondering a bit more on the "how to enable default deny" case. From the Group posting we landed on:


We could add a "policyTypes" field indicating what kinds of traffic the 
policy affects. And if it's unspecified, it would get defaulted to: 

  - [ "ingress", "egress" ] if the policy has both ingress: and egress: 
    sections 

  - [ "egress" ] if the policy has an egress: section but no ingress: 
    section 

  - [ "ingress"] if the policy has an ingress: section but no egress: 
    section, or, for backward compatibility, if it has neither 
    ingress: nor egress:.

Instead of adding something to NetworkPolicySpec, should we augment the Egress rule with a field that resembles the default? Then we can consider deprecating the current Ingress defaults over a few releases in favor of a similar change.

Ex:

// NetworkPolicyEgressRule describes a particular set of traffic that pods can egress to
// matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to.
type NetworkPolicyEgressRule struct {
	// List of destination ports for outgoing traffic.
	//Each item in this list is combined using a logical OR. If this field is
	// empty or missing, this rule matches all ports (traffic not restricted by port).
	// If this field is present and contains at least one item, then this rule does note
	// constrain outgoing traffic.
	// +optional
	Ports []NetworkPolicyPort

	// List of sources for outgoing traffic of pods selected for this rule.
	// Items in this list are combined using a logical OR operation. If this field is
	// empty or missing, this rule matches all sources (traffic not restricted by
	// source). If this field is present and contains at least on item, this rule
	// allows traffic only if the traffic matches at least one item in the from list.
	// +optional
	To []NetworkPolicyPeer

        // This is the default EgressRule applied to pods across a cluster.
        // Options are Permit or Prohibit
        // +optional
        DefaultPolicy: 
}

danwinship · 2017-08-28T13:47:12Z

We can't have "DefaultPolicy" in NetworkPolicyEgressRule, because what if there were two rules with opposite default policies?

But maybe there could be something like a "Nothing" option in NetworkPolicyEgressRule/NetworkPolicyIngressRule so you can explicitly write a rule that doesn't match anything:

kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
metadata:
  name: default-deny-egress
spec:
  podSelector:
  egress:
  - to:
    - nothing:

and that combines fine with other policies without contradiction.

Although "Nothing" probably isn't the right name, and it can't be just an empty field because that would get lost in serialization.

cmluciano · 2017-08-28T16:33:44Z

/retest

cmluciano · 2017-08-28T16:47:33Z

I like the option of adding to the Peer group, however I'm also lost on what to name it.

ahmetb · 2017-08-28T17:13:03Z

Is there a design/proposal doc for egress rules in networkpolicy? Or is the comment #51351 (comment) sufficient enough to capture the gist of this work?

cmluciano · 2017-08-28T17:44:07Z

@ahmetb The summary of proposals is in a google doc.

SIG-Network Mailing List Post: egress network policy requirements

danwinship · 2017-08-28T18:31:05Z

So actually, wait, the problem is not "how to enable default deny", it's "how to not (accidentally) enable default deny". We told people to use a policy like this for ingress default-deny in 1.7:

kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
spec:
  podSelector:

But that will now implicitly have an empty egress rule array. So the solution has to be that, at least in some cases, an empty/non-existent egress section does not mean "deny all egress".

My suggestion had been to fix that by adding a way to specify that egress is ignored for a given policy (and make that the default behavior for existing policies).

I guess the other way would be to say that empty/non-existent egress never means "deny all egress", it means "allow all egress" (and you have to include an explicit don't-allow-anything NetworkPolicyEgressRule if you want to deny-all). But that's pretty egregiously different from ingress behavior.

cmluciano · 2017-08-28T18:38:27Z

I'm fine with the PolicyType field for now if we want to maintain the Ingress behavior. I figured it was worth bringing up since the Default code that I started writing looked messy due to all of the edge-cases.

aanm · 2017-08-28T22:49:12Z

pkg/apis/networking/types.go

@@ -77,6 +86,26 @@ type NetworkPolicyIngressRule struct {
 	From []NetworkPolicyPeer
 }

+// NetworkPolicyEgressRule describes a particular set of traffic that is allowed out of pods
+// matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to.


Small nit: The traffic must match both Ports and To

@cmluciano this comment is still pending it looks like

JSON names are correct

aanm · 2017-08-28T22:49:53Z

pkg/apis/networking/types.go

+	// source). If this field is present and contains at least on item, this rule
+	// allows traffic only if the traffic matches at least one item in the from list.
+	// +optional
+	To []NetworkPolicyPeer


in the to list. -> in the To list.

aanm · 2017-08-29T00:21:34Z

This egress policy is enforced on connections made from the pod but what about on reply packets?

Example:

an ingress policy for app=database to receive requests from app=frontend
an egress policy for app=database that is only allowed to send traffic to the app=backup

Based on the egress policy installed, can app=database reply to app=frontend?

thockin · 2017-08-29T01:31:58Z

We can't really deprecate the ingress defaults until we rev the API to v2{alpha,beta,}. It is part of the API semantic. Unfortunately, lack of ingress means no ingress, but we don't want lack of egress to mean no egress (that would be a breaking change). We should have demanded ingress be specified to mean "no ingress". We botched that. The policyTypes field is the remedy.

We might be able to slightly rectify the situation, though. I'm going to think out loud:

We have IPBlock now, which can unambiguously indicate "nothing" as 0.0.0.0/32 What if we:

change the defaulting logic for NetworkPolicySpec to something like:

if len(spec.Ingress) == 0 {
  nothing := new(IPBlock)
  nothing.CIDR = "0.0.0.0/32"
  spec.Ingress = []NetworkPolicyIngressRule{{From: []NetworkPolicyPeer{{IPBlock: nothing}}}}
}

add egress, demand that "deny all" specify to: { ipblock: { cidr: "0.0.0.0/32" }}
change the above default logic to if len(spec.Ingress) + len(spec.Egress) == 0 {

If user specifies nothing, they get the default (no ingress).

If the user specifies ingress but not egress, they get their ingress rules

If the user specifies egress but not ingress, they get their egress rules

If the use specifies ingress and egress, they get their ingress and egress rules

The only thing that is irregular is the defaulting logic. Does this break anything? I am trying to work through it. If a user creates this on v1.8, then rolls back to v1.7, the ipBlock will get dropped, leaving an empty from which fails validation. Damn. We could change the validation in v1.8 and do the above in v1.9.

Starting over:

We could just leave the empty ingress to be irregular. Can we only make it irregular iff egress is not specified?

If the user specifies nothing, they get today's behavior (no ingress).

If the user specifies ingress but not egress, they get their ingress rules

If the user specifies egress but not ingress, they get their egress rules and ingress is unaffected

If the use specifies ingress and egress, they get their ingress and egress rules

Would this break anything? If a user had an empty NetworkPolicy object, and they add egress to it, it would stop being a no-ingress rule. That's bad.

Starting over:

What if we change the defaulting for NetworkPolicyPeer and allow all-nil to mean "none". That would be a breaking change upon rollback.

Starting over:

We're stuck. We can't even do what Dan suggested. I think we have to respect that any NP object that has no ingress block means "default-deny ingress". That means the policyTypes list has to always default to containing "Ingress". The only way it can not affect ingress is if the user specifies just "Egress".

Someone check my math or convince me I am wrong?

danwinship · 2017-08-29T13:19:26Z

This egress policy is enforced on connections made from the pod but what about on reply packets?

Reply packets are always allowed, just like with ingress policy. (ie, if you have an ingress policy for app=database to receive requests from app=frontend, then that also allows replies from the database pod to the frontend pod, even though it doesn't allow the database pod to initiate connections to the frontend pod.)

danwinship · 2017-08-29T14:03:15Z

We're stuck. We can't even do what Dan suggested. I think we have to respect that any NP object that has no ingress block means "default-deny ingress". That means the policyTypes list has to always default to containing "Ingress". The only way it can not affect ingress is if the user specifies just "Egress".

Someone check my math or convince me I am wrong?

Didn't you just contradict yourself? "any NP object that has no ingress block means 'default-deny ingress'" vs "The only way it can not affect ingress is if the user specifies just 'Egress'" ?

So like:

kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
spec:
  policyTypes: [ "Egress" ]
  egress:

Does that default-deny ingress or not?

If the concern is only with rollback, then it seems a little weird to worry about the ingress behavior of that policy after rolling back from 1.8 to 1.7, since regardless of the interpretation of ingress, the policy as a whole is guaranteed to not have the behavior the user wanted anyway...

If we need to make it possible to roll back and not have egress-only policies turn into deny-ingress policies then I guess the only way we can do that is to say that there's no such thing as an egress-only policy. So the fully foward-and-backward-compatible rule would be:

Any pod that is selected by any NetworkPolicy is deny-ingress, and only allows incoming connections that match the ingress clause of some NetworkPolicy that selects it. (Unchanged from 1.7; for rollback compatibility we have to ensure that any policy in 1.8 still has the same ingress semantics it would have had in 1.7 if you remove any new fields.)
Any pod that is selected by a NetworkPolicy that contains an egress clause is deny-egress, and only allows outgoing connections that match the egress clause of some NetworkPolicy that selects it. (For forward compatibility with existing 1.7 policies, we can't have egress be affected by policies that don't explicitly mention egress.)
To make an egress default-deny policy, you can do:
```
kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
spec:
  podSelector:
  egress:
  - ipBlock:
      cidr: 0.0.0.0/0
```
except that that's also a default-deny ingress policy too; there's no way to write a policy that just denies egress without affecting ingress. Which is terrible.

Actually, there's another way around the problem, which is if we say that podSelector only describes the pods affected by the ingress clause, and have a separate egressPodSelector to describe the pods that are affected by the egress section of the policy. And then you could write:

spec:
    egressPodSelector:
      matchLabels:
        app: frontend
    egress:    # empty == denied
    podSelector:
      matchLabels:
        asdfasdfasdfa: asdfasdfasdfasdfasdfas

or if you want to be more paranoid/explicit:

    podSelector:
      matchExpressions:
      - { key: dummy, operator: Exists }
      - { key: dummy, operator: NotExists }

either of which is also terrible, but maybe someone can come up with a less terrible version? (Or maybe we just let v1 be terrible but plan to move to a less-terrible v2 soon?)

cmluciano · 2017-08-29T16:30:14Z

@danwinship @thockin @caseydavenport Take a look at f058d5d . The tests will probably fail since I have not added this to v1beta1, but I wanted to see if this behavior works.

thockin · 2017-08-29T19:54:58Z

Didn't you just contradict yourself? "any NP object that has no ingress block means 'default-deny ingress'" vs "The only way it can not affect ingress is if the user specifies just 'Egress'" ?

I don't think so. I just wasn't clear. If policyTypes is explicitly [ "Egress" ] and nothing else, then the ingress block has no effect. If the policyTypes field is not specified, it MUST be defaulted to at least [ "Ingress" ], though I think it is safe to detect non-empty egress block and add "Egress" to policyTypes.

kind: NetworkPolicy
apiVersion: networking.k8s.io/v1
spec:
  policyTypes: [ "Egress" ]
  egress:

Does that default-deny ingress or not?

It would have no effect on Ingress. But you're right - upon a rollback, this breaks the cluster. Both policyTypes and egress would be ignored in 1.7, converting it to a default-deny ingress rule.

for rollback compatibility we have to ensure that any policy in 1.8 still has the same ingress semantics it would have had in 1.7 if you remove any new fields.

This is something I think I never really considered about one-of blocks. You can't add a new option and then allow rollbacks. Any IPBlock user who rolls back to 1.7 will be left with an object that fails validation. The NetworkPolicy "foobar2" is invalid: spec.ingress[0].from[0]: Required value: must specify a from type. We've already introduced a breaking change by adding IPBlock at all.

I need to think about how we do this elsewhere. It may be that we're setting the bar too high here.

cmluciano · 2017-09-01T22:15:20Z

@thockin @danwinship @caseydavenport

This should pass all tests now.

TODO:

Need to validate empty policyTypes
Duplicate types
Refactor validation pieces (split out ports)

Can we have these as TODOs post-freeze?

caseydavenport · 2017-09-01T22:45:40Z

I suspect we can make the validation tweaks post-freeze if we really need to.

I'd like to get a conclusion on this comment though: https://github.com/kubernetes/kubernetes/pull/51351/files#r136650232

@thockin @danwinship

thockin

Mostly nits or code layout, not semantics.

thockin · 2017-09-01T22:35:24Z

pkg/apis/extensions/v1beta1/defaults.go

@@ -133,4 +133,14 @@ func SetDefaults_NetworkPolicy(obj *extensionsv1beta1.NetworkPolicy) {
 			}
 		}
 	}
+
+	if obj.Spec.PolicyTypes == nil || len(obj.Spec.PolicyTypes) == 0 {


+1, fixup in followup OK

thockin · 2017-09-01T22:44:45Z

pkg/apis/extensions/v1beta1/defaults.go

+
+	if obj.Spec.PolicyTypes == nil || len(obj.Spec.PolicyTypes) == 0 {
+		// Default for undefined PolicyTypes with defined Egress rules
+		if obj.Spec.Egress != nil || len(obj.Spec.Egress) != 0 {


same, just len()

thockin · 2017-09-01T22:49:29Z

pkg/apis/extensions/v1beta1/defaults.go

+	if obj.Spec.PolicyTypes == nil || len(obj.Spec.PolicyTypes) == 0 {
+		// Default for undefined PolicyTypes with defined Egress rules
+		if obj.Spec.Egress != nil || len(obj.Spec.Egress) != 0 {
+			obj.Spec.PolicyTypes = []extensionsv1beta1.PolicyType{extensionsv1beta1.PolicyTypeIngress, extensionsv1beta1.PolicyTypeEgress}


yes. I would restructure it as:

if len(obj.Spec.PolicyTypes) == 0 { // Any policy that does not specify policyTypes implies at least "Ingress". obj.Spec.PolicyTypes = []extensionsv1beta1.PolicyType{extensionsv1beta1.PolicyTypeIngress} if len(obj.Spec.Egress) != 0 { obj.Spec.PolicyTypes = append(obj.Spec.PolicyTypes, extensionsv1beta1.PolicyTypeEgress) } }

thockin · 2017-09-01T22:53:03Z

pkg/apis/networking/types.go

+	// allowed if there are no NetworkPolicies selecting the pod (and cluster policy
+	// otherwise allows the traffic), OR if the traffic matches at least one egress rule
+	// across all of the NetworkPolicy objects whose podSelector matches the pod. If
+	// this field is empty then this NetworkPolicy does not limit any outgoing traffic.


Did we agree that empty hear means allow-all while empty ingress means deny-all? I can't recall why? It should be the same or well-documented here, why not.

I don't think we did - this should be the same as for ingress.

thockin · 2017-09-01T22:54:39Z

pkg/apis/networking/types.go

+	// (whether or not they contain an Ingress section) are assumed to affect Ingress.
+	// If you want to write an egress-only policy, you must explicitly specify PolicyTypes [ "Egress" ].
+	// Likewise, if you want to write a policy that specifies that no egress is allowed,
+	// you must specify a PolicyTypes values that include "Egress" (since such a policy would not include


s/values/value

s/PolicyType/policyType - JSON name

thockin · 2017-09-01T22:56:02Z

pkg/apis/networking/types.go

@@ -77,6 +86,26 @@ type NetworkPolicyIngressRule struct {
 	From []NetworkPolicyPeer
 }

+// NetworkPolicyEgressRule describes a particular set of traffic that is allowed out of pods
+// matched by a NetworkPolicySpec's podSelector. The traffic must match both ports and to.


JSON names are correct

thockin · 2017-09-01T23:01:07Z

pkg/apis/extensions/v1beta1/defaults.go

+	if obj.Spec.PolicyTypes == nil || len(obj.Spec.PolicyTypes) == 0 {
+		// Default for undefined PolicyTypes with defined Egress rules
+		if obj.Spec.Egress != nil || len(obj.Spec.Egress) != 0 {
+			obj.Spec.PolicyTypes = []extensionsv1beta1.PolicyType{extensionsv1beta1.PolicyTypeIngress, extensionsv1beta1.PolicyTypeEgress}


@caseydavenport If we don't assume Ingress, then any policy that is a default-deny-ingress today would change meaning by adding an egress block. That's not compatible.

thockin · 2017-09-01T23:03:44Z

pkg/apis/networking/validation/validation.go

+		}
+	}
+	// Validate PolicyTypes
+	if len(spec.PolicyTypes) > 2 {


why not convert it to a sets.String, and check that the set only contains known items? I don't like coding in numbers like 2.

thockin · 2017-09-01T23:06:34Z

pkg/apis/networking/validation/validation.go

+	for i, pType := range spec.PolicyTypes {
+		policyPath := fldPath.Child("policyTypes").Index(i)
+		if pType != networking.PolicyTypeIngress && pType != networking.PolicyTypeEgress {
+			allErrs = append(allErrs, field.NotSupported(policyPath, pType, []string{string("Ingress"), string("Egress")}))


should use teh const values.

allowed := sets.NewString(PolicyTypeIngress, PolicyTypeEgress) for _, p := range spec.PolicyTypes { if !allowed.Has(string(p)) { // error

thockin · 2017-09-01T23:08:15Z

/lgtm
/approve

http://gph.is/1y9uxL8

fejta-bot · 2017-09-02T02:42:53Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-09-02T12:09:51Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-09-02T20:33:51Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-09-03T04:36:52Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

Signed-off-by: Christopher M. Luciano <cmluciano@us.ibm.com>

cmluciano · 2017-09-05T17:21:22Z

Since I needed to rebase and fix some golint stuff, I added the comment and validation suggestions as well. Will need another LGTM

cmluciano · 2017-09-05T17:42:29Z

/test pull-kubernetes-unit

thockin · 2017-09-06T00:03:35Z

/lgtm

k8s-github-robot · 2017-09-06T00:03:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cmluciano, thockin

Associated issue: 50453

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~api/OWNERS~~ [thockin]
~~docs/OWNERS~~ [thockin]
~~hack/OWNERS~~ [thockin]
~~pkg/api/OWNERS~~ [thockin]
~~staging/src/k8s.io/api/OWNERS~~ [thockin]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

caseydavenport · 2017-09-06T00:14:44Z

/test pull-kubernetes-e2e-kops-aws

k8s-ci-robot · 2017-09-06T00:55:50Z

@cmluciano: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-kops-aws	`84290ce`	link	`/test pull-kubernetes-e2e-kops-aws`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-github-robot · 2017-09-06T01:40:25Z

Automatic merge from submit-queue (batch tested with PRs 51984, 51351, 51873, 51795, 51634)

cmluciano added the sig/network Categorizes an issue or PR as relevant to SIG Network. label Aug 25, 2017

cmluciano added this to the v1.8 milestone Aug 25, 2017

cmluciano assigned danwinship, caseydavenport and thockin Aug 25, 2017

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 25, 2017

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Aug 25, 2017

danwinship suggested changes Aug 25, 2017

View reviewed changes

thockin reviewed Aug 26, 2017

View reviewed changes

cmluciano force-pushed the cml/addegressrule branch from 2a2cd2b to 6885dfb Compare August 28, 2017 14:46

aanm reviewed Aug 28, 2017

View reviewed changes

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 29, 2017

thockin reviewed Sep 1, 2017

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 1, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 1, 2017

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 3, 2017

Christopher M. Luciano added 3 commits September 5, 2017 12:01

Add EgressRule to NetworkPolicy

b03302f

Signed-off-by: Christopher M. Luciano <cmluciano@us.ibm.com>

Add PolicyTypes to NetworkPolicy Spec

90b1398

Signed-off-by: Christopher M. Luciano <cmluciano@us.ibm.com>

EgressRule generated code

84290ce

cmluciano force-pushed the cml/addegressrule branch from e97db92 to 84290ce Compare September 5, 2017 17:10

k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 5, 2017

cmluciano mentioned this pull request Sep 5, 2017

Egress support for Network Policy kubernetes/enhancements#366

Closed

0xmichalis mentioned this pull request Sep 5, 2017

Set owner references in pods owned by ProwJobs kubernetes/test-infra#3409

Closed

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 6, 2017

k8s-github-robot merged commit 6430a27 into kubernetes:master Sep 6, 2017

cmluciano mentioned this pull request Sep 11, 2017

document egress networkpolicy support kubernetes/website#5393

Closed

redbaron mentioned this pull request Sep 17, 2017

Move flanneld + calico setup to use Kubernetes storage kubernetes-retired/kube-aws#909

Closed

Add EgressRule to NetworkPolicy #51351

Add EgressRule to NetworkPolicy #51351

Conversation

cmluciano commented Aug 25, 2017 • edited by thockin

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmluciano Aug 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmluciano Aug 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented Aug 26, 2017

cmluciano commented Aug 26, 2017

danwinship commented Aug 28, 2017

cmluciano commented Aug 28, 2017

cmluciano commented Aug 28, 2017

ahmetb commented Aug 28, 2017

cmluciano commented Aug 28, 2017

danwinship commented Aug 28, 2017

cmluciano commented Aug 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aanm commented Aug 29, 2017

thockin commented Aug 29, 2017

danwinship commented Aug 29, 2017

danwinship commented Aug 29, 2017

cmluciano commented Aug 29, 2017

thockin commented Aug 29, 2017

cmluciano commented Sep 1, 2017

caseydavenport commented Sep 1, 2017 • edited

thockin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thockin commented Sep 1, 2017

fejta-bot commented Sep 2, 2017

fejta-bot commented Sep 2, 2017

fejta-bot commented Sep 2, 2017

fejta-bot commented Sep 3, 2017

cmluciano commented Sep 5, 2017

cmluciano commented Sep 5, 2017

thockin commented Sep 6, 2017

k8s-github-robot commented Sep 6, 2017

caseydavenport commented Sep 6, 2017

k8s-ci-robot commented Sep 6, 2017

k8s-github-robot commented Sep 6, 2017

cmluciano commented Aug 25, 2017 •

edited by thockin

cmluciano Aug 26, 2017 •

edited

cmluciano Aug 26, 2017 •

edited

caseydavenport commented Sep 1, 2017 •

edited